refactor: remove legacy API server and localize Nix sibling deps - #61
Conversation
Drop the unused internal API server wiring, update the changelog to match the current daemon surface, and make Nix builds resolve sibling modules locally.
Patel230
left a comment
There was a problem hiding this comment.
Net -422 LOC and the API server + its tests are cleanly excised with no dangling references — solid. Flake.nix change makes the build hermetic against the stale Go proxy, which mirrors the Dockerfile's behavior.
Approving with one must-follow-up:
flake.nixsetupReplace(>> go.mod) is not idempotent acrosspreBuildandoverrideModAttrs.preBuild. If the flake is re-evaluated between phases, sibling replace directives will be appended twice andgo modwill error. Fix by stripping existing sibling replace directives before appending, or by readinggo.modinto a string, replacing vialib.replaceStrings, and writing once. Also worth a comment thatdirOfassumes thegithub.com/<org>/<name>shape.
Smaller items:
cmd/hawk/main.gostill wirescmd.SetVersion/mcp.SetClientVersion/sandbox.ContainerImageTag = Versionindependently. Withapi.SetVersiongone, a singlecmd.RegisterVersionhelper would be a nice follow-up.- Consider adding a
nix flake checkstep to CI so futureflake.nixchanges are validated by more than cross-compile builds. - CHANGELOG entry mixes two unrelated changes; either two lines or two commits for clean revert surface.
CI is green; Docker build-and-push + fuzz are still pending but unrelated to the diff.
Strip any existing sibling replace directives before appending them so the preBuild logic can run multiple times without duplicating go.mod entries.
Patel230
left a comment
There was a problem hiding this comment.
Net -422 LOC and the API server + its tests are cleanly excised with no dangling references — solid. Flake.nix change makes the build hermetic against the stale Go proxy, which mirrors the Dockerfile's behavior.
Approving with one must-follow-up:
flake.nixsetupReplace(>> go.mod) is not idempotent acrosspreBuildandoverrideModAttrs.preBuild. If the flake is re-evaluated between phases, sibling replace directives will be appended twice andgo modwill error. Fix by stripping existing sibling replace directives before appending, or by readinggo.modinto a string, replacing vialib.replaceStrings, and writing once. Also worth a comment thatdirOfassumes thegithub.com/<org>/<name>shape.
Smaller items:
cmd/hawk/main.gostill wirescmd.SetVersion/mcp.SetClientVersion/sandbox.ContainerImageTag = Versionindependently. Withapi.SetVersiongone, a singlecmd.RegisterVersionhelper would be a nice follow-up.- Consider adding a
nix flake checkstep to CI so futureflake.nixchanges are validated by more than cross-compile builds. - CHANGELOG entry mixes two unrelated changes; either two lines or two commits for clean revert surface.
CI is green; Docker build-and-push + fuzz are still pending but unrelated to the diff.
|
Self-review notes (cannot self-approve or self-request-changes via gh): Net -422 LOC and the API server + its tests are cleanly excised — solid. Flake.nix makes the build hermetic against the stale Go proxy. Must-fix before merge:
Smaller items:
CI all green on the most recent run (Docker fuzz and builds re-triggered after this comment). |
* refactor: remove legacy API server and localize nix sibling deps Drop the unused internal API server wiring, update the changelog to match the current daemon surface, and make Nix builds resolve sibling modules locally. * fix: make nix sibling replace setup idempotent Strip any existing sibling replace directives before appending them so the preBuild logic can run multiple times without duplicating go.mod entries.
* refactor: remove legacy API server and localize nix sibling deps Drop the unused internal API server wiring, update the changelog to match the current daemon surface, and make Nix builds resolve sibling modules locally. * fix: make nix sibling replace setup idempotent Strip any existing sibling replace directives before appending them so the preBuild logic can run multiple times without duplicating go.mod entries.
* feat(skills): add threat scan, lockfile pinning, manifest hardening - internal/plugin/threatscan.go: threat-pattern scanner (command injection, data exfiltration, encoded payloads, network calls) with 0-100 scoring; installs are refused below the block threshold - internal/plugin/skillslock.go: per-scope skills-lock.json pinning source repo, cloned commit, and content SHA-256; maintained on install and remove - internal/plugin/manifest_hardening.go: bounded symlink-refusing manifest reads with duplicate-JSON-key rejection and strict name and semver patterns, wired into ParseManifestV2 and ValidateV2 - docs: install-time security and lockfile section in user guide * chore(deps): bump external/yaad to c968f17 (docs PR #61) * chore(deps): align yaad module version with submodule bump
Summary
Test plan
Made with Cursor